Upgrade Docxodus to v7.0.0 and expose its comparison-engine flag#25
Merged
Conversation
Docxodus v7.0.0 retargets to net10.0. The root global.json governs SDK selection for builds run from the repo root, so it moves to 10.0.0; the three CI dotnet-version pins follow. The ooxmlpowertools project stays on net8.0 and the .NET 10 SDK builds it self-contained without complaint, so only one SDK is provisioned. Because two target frameworks are now in play, build_differ.py carries an explicit per-engine tfm rather than a hardcoded net8.0 publish path.
Docxodus v7.0.0 added --engine=wmlcomparer|docxdiff. Surface it as a kwarg, normalized the way the CLI's own TryParseEngine normalizes it. Omitting the kwarg emits no flag at all, so the default argv is unchanged. The docxdiff branch of DocxCompare.ToDocxDiffSettings drops detail_threshold, simplify_move_markup, and detect_format_changes, and the CLI accepts them there without a diagnostic. Rejecting them on key presence -- whatever the value -- keeps that from becoming a Python-level footgun: a caller who tunes granularity under docxdiff would otherwise get untuned output and no signal.
wmlcomparer finds 9 revisions and docxdiff finds 11 on the same pair. The counts differ because the algorithms differ, so both are asserted: the 9 guards against an accidental default flip, and the 11 against a silent no-op where --engine failed to reach the binary.
Bump to 0.3.0: the vendored engine jumps v5.4.2 -> v7.0.0 and gains a comparison algorithm, and contributors now need a .NET 10 SDK to build it at all. Records that docxdiff reports a different revision count than wmlcomparer on identical input -- expected, not a defect.
The warning told every caller to pair detect_moves with simplify_move_markup, but docxdiff honours the former and rejects the latter, so a docxdiff user following the docs hit a ValueError with no available mitigation. Scope the warning and say what docxdiff does instead -- without claiming it is free of the Word bug, which is untested. test_docxodus_explicit_wmlcomparer_matches_default now parses and compares the two revision counts rather than asserting each contains "9" independently, so the test checks the equivalence its name promises.
6562725 to
bbc829e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upgrades the vendored Docxodus submodule from v5.4.2 to v7.0.0 and exposes v7.0.0's new
--engine=wmlcomparer|docxdiffselector as anengine=kwarg onDocxodusEngine.wmlcomparerstays the default. Whenengineis omitted no--engineflag is emitted at all,so the argv is byte-identical to the pre-v7 one.
The upgrade drags .NET 10 in with it
Docxodus v7.0.0 retargets
net8.0→net10.0. That reaches past the submodule:global.json→ SDK10.0.0. It governs SDK selection becausedotnetresolvesglobal.jsonfrom the working directory, not the project path, sodocxodus/global.jsonis never consulted by our builds.
dotnet-versionpins →10.0.x.build_differ.pyhardcodednet8.0in its publish path. The two engines now target differentframeworks, so
ENGINEScarries a per-enginetfm.Only one SDK is provisioned. The .NET 10 SDK builds the
net8.0ooxmlpowertools projectself-contained; its binary still reports
Revisions found: 9. Verified locally onlinux-x64and cross-built for
linux-arm64. The Windows and macOS wheel builds are exercised for thefirst time by this PR's CI.
Contributors now need a .NET 10 SDK — a stock .NET 8 box can no longer build the Docxodus
engine at all. Documented in the README and developer guide.
docxdiff silently drops three settings, so Python rejects them
On the
docxdiffbranch,DocxCompare.ToDocxDiffSettingsmaps only a subset ofWmlComparerSettings. It dropsdetail_threshold,simplify_move_markup, anddetect_format_changes— the CLI accepts them, prints no diagnostic, and produces output as ifthey were never passed. Confirmed empirically:
--engine=docxdiff --detail-threshold=0.9yields the same result as
--engine=docxdiffalone.Passing any of those three alongside
engine="docxdiff"now raisesValueErrorbefore we shellout. Rejection is on key presence, regardless of value: two of the three are no-ops at their
defaults, so a value-sensitive rule would raise for
detect_format_changes=Falsebut not fordetect_format_changes=True— a worse contract than "naming a WmlComparer-only knob whileasking for docxdiff is an error."
docxdiffdoes honourdetect_moves,case_insensitive,conflate_spaces,move_similarity_threshold,move_minimum_word_count, anddate_time.The two engines report different revision counts
On
tests/fixtures/,wmlcomparerfinds 9 revisions anddocxdifffinds 11. Differentalgorithms, both correct. Both counts are asserted: the 9 guards against an accidental default
flip, the 11 against a silent no-op where
--enginenever reached the binary.Also scoped the README's move-detection warning to WmlComparer. It told every caller to pair
detect_moveswithsimplify_move_markup, butdocxdiffhonours the former and rejects thelatter, so a docxdiff user following the docs hit a
ValueErrorwith no available mitigation.DocxDiff renders moves natively; whether Word's ID-collision bug affects its move markup is
untested and the docs no longer imply either way.
Testing
40 tests pass. Version bumped
0.2.1→0.3.0(the bump also re-keys the binary-extractioncache, which is keyed by package version).